Skip to content

Conversation

sergey-miryanov
Copy link
Contributor

@sergey-miryanov sergey-miryanov commented Jun 23, 2025

According to discussion Py_TPFLAGS_MANAGED_WEAKREF and Py_TPFLAGS_MANAGED_DICT must be used only if Py_TPFLAGS_HAVE_GC is set.


📚 Documentation preview 📚: https://cpython-previews--135863.org.readthedocs.build/

@sergey-miryanov
Copy link
Contributor Author

@markshannon Please take a look.

@chris-se
Copy link

As the person who reported #134786: I would suggest making the documentation more explicit and replacing should with must here, especially since the code now errors out if Py_TPFLAGS_HAVE_GC is not set.

@sergey-miryanov sergey-miryanov marked this pull request as draft July 26, 2025 18:47
@Starbuck5
Copy link

As the person who reported #134786: I would suggest making the documentation more explicit and replacing should with must here, especially since the code now errors out if Py_TPFLAGS_HAVE_GC is not set.

+1

Additionally, this PR puts this into a WhatsNew entry for 3.15. This is not a new thing, this is a documentation fix + fix for the runtime to not allow these invalid types to be created. So I think the added whatsnew is misleading.

@sergey-miryanov
Copy link
Contributor Author

I'm not sure if we should support Py_TPFLAGS_MANAGED_DICT without Py_TPFLAGS_HAVE_GC, because Py_TPFLAGS_MANAGED_DICT implies Py_TPFLAGS_HAVE_GC. However, I have added it, though, to confirm that it is doable.

I think the current version needs some updates and clarification in the docs. First, though, I would like to get some feedback on the overall approach. Is this the right way to do after all?

Please take a look if someone interested.

@sergey-miryanov sergey-miryanov marked this pull request as ready for review September 25, 2025 19:42
@sergey-miryanov sergey-miryanov changed the title gh-134786: Py_TPFLAGS_MANAGED_WEAKREF implies Py_TPFLAGS_HAVE_GC too and force checking of its presence gh-134786: Allow use Py_TPFLAGS_MANAGED_WEAKREF without Py_TPFLAGS_HAVE_GC Sep 26, 2025
@sergey-miryanov sergey-miryanov changed the title gh-134786: Allow use Py_TPFLAGS_MANAGED_WEAKREF without Py_TPFLAGS_HAVE_GC gh-134786: Allow to use Py_TPFLAGS_MANAGED_WEAKREF without Py_TPFLAGS_HAVE_GC Sep 26, 2025
@sergey-miryanov
Copy link
Contributor Author

@markshannon Please take a look.

Copy link
Contributor

@efimov-mikhail efimov-mikhail left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should rename this PR?

Comment on lines +791 to +792
* If the :c:macro:`Py_TPFLAGS_MANAGED_DICT` and :c:macro:`Py_TPFLAGS_MANAGED_WEAKREF`
flags are set then :c:macro:`Py_TPFLAGS_HAVE_GC` must be set too.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* If the :c:macro:`Py_TPFLAGS_MANAGED_DICT` and :c:macro:`Py_TPFLAGS_MANAGED_WEAKREF`
flags are set then :c:macro:`Py_TPFLAGS_HAVE_GC` must be set too.
* If the :c:macro:`Py_TPFLAGS_MANAGED_DICT` or :c:macro:`Py_TPFLAGS_MANAGED_WEAKREF`
flag is set then :c:macro:`Py_TPFLAGS_HAVE_GC` must be set too.

Maybe it should be "or", not "and"?

@sergey-miryanov
Copy link
Contributor Author

Maybe we should rename this PR?

I forgot to apply changes :(

@sergey-miryanov sergey-miryanov changed the title gh-134786: Allow to use Py_TPFLAGS_MANAGED_WEAKREF without Py_TPFLAGS_HAVE_GC gh-134786: Py_TPFLAGS_MANAGED_WEAKREF and Py_TPFLAGS_MANAGED_DICT must be used only if Py_TPFLAGS_HAVE_GC set Sep 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants